Hey @kiminuo, can you amend the commit and remove my mention? Keep in mind that mentions in commits lead to notifications. Ty.

Hey @kiminuo, can you amend the commit and remove my mention? Keep in mind that mentions in commits lead to notifications. Ty.

Yes, sorry for that.

Thanks for picking this up.

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #26525 (Remove -mempoolfullrbf option by BitcoinErrorLog)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

Concept ACK. For the test commit, maybe a more descriptive title that can be understood on its own.

@jonatack Thanks for the superb review! I'm working on incorporating your suggestions.

It would be good if the functional test actually verified that the values are correctly calculated after creating a few txns; ATM it is only really a smoke test that verifies the output structure.

This is on my TODO list. So I will improve it over coming days.

How can we expect the Bitcoin Core wallet to behave reasonably if it can't predict the fees well?

The Bitcoin Core wallet does not use this feature?

The original idea of this patch was to add a small and easy to maintain efficient way to generate a fee histogram to bitcoin core (see original PR description #15836 (comment)).

As described there, it was always possible to use getrawmempool (@jhoenicke script) to achieve this goal.

However, it is not efficient to dumb out all transactions of the complete mempool in a json format just to calculate the fee histogram with a python script.

Thus I think this is a valuable addition to Bitcoin Core.

The patch size is minimal and it seems like there is some interest in this feature.

EDIT: this PR has nothing to do with the Bitcoin cores internal wallet. It's pure mempool statistics.

How can we expect the Bitcoin Core wallet to behave reasonably if it can't predict the fees well?

The Bitcoin Core wallet does not use this feature?

Does not use yet?

The Bitcoin Core wallet does not use this feature?

My point is that a wallet without fee estimation is not very usable and harms the overall network. If we don't want to have an effective way of determining the fee in Bitcoin Core, we might as well remove the entire wallet from the codebase.

There seems to be confusion here. This PR does not touch the fee estimator or the wallet in any way. The fee estimator already keeps track of how many transactions at specific feerates {in the mempool, have confirmed within N blocks, haven't confirmed within N blocks} there are internally; the wallet queries it directly for feerate estimates. I don't know of any examples where the wallet needs to see a graph of something.

If you feel that the fee estimation algorithms need improvement, by all means I'd say some review/PRs there would be welcome. Maybe take a look at #25380 or #21161.

For those that feel very passionately about improving how the wallet calculates fees, my personal recommendation would be to review #26152 to have the strongest impact.

It's confusing to me why this PR, tagged as [RPC/REST/ZMQ] and only touching src/rest.cpp and files in src/rpc/*, should have anything to do with the wallet or the GUI. AFAIK the wallet doesn't use the RPC interface to communicate with bitcoind. If the wallet or the GUI would want a histogram, it couldn't use the code added in this PR.

edit: glozow was 1min faster

If the wallet or the GUI would want a histogram, it couldn't use the code added in this PR.

That would require just moving fee histogram code out of MempoolInfoToJSON() to a separate function somewhere out of rpc/, code itself seems to me reusable.

Bitcoin Core has had fee estimation logic for a long time (estimatefee RPC was added in 2014, replaced with estimatesmartfee in 2015).

The Bitcoin Core wallet automatically uses this internal feerate estimation logic to decide on the fees of transactions. You need to override things, or use more low-level RPCs, to not use it.

That feerate estimation logic works by looking at how quickly mempool transactions get confirmed, over longer time windows, not by looking at the current mempool composition (looking at just your own mempool may give very bad estimates if your relay policy isn't exactly what the network and miners use, which is very hard to guarantee).

This PR is about exposing mempool feerate histogram data through RPC. It's not a feerate estimator, and is entirely unrelated to the wallet. While the logic could in theory be used for feerate estimation too, that's not what this PR does, and I'm concerned that doing so would risk giving very inaccurate results when faced with diverging network policies.

Conceptually, however, I think it's reasonable to add something like this, as just RPC (or GUI) output. It can be informative for users, even if it can't be directly used for automatic fee estimation. Of course, that's subject to reviewer interest.

I am not against sipa as I understand fee estimation is difficult however estimatesmartfee sucks and could be improved.

There are lot of occasions when I found it to be irrelevant and just look at mempool.space or multiple explorers

One of the project that I found more useful: https://github.com/TrueLevelSA/btc-congestion-manager

Anyway, this PR tries to improve how projects use mempool info and should not have been controversial at any point.

Request to all developers including maintainers to re consider their opinion.

For Wasabi's purposes this output is needed purely for fee estimation, so it'd be ideal if Core would estimate fees properly. Here's how I see things:

• estimatesmartfee estimates fees based on the PAST
• This PR enables us to put minimums on Core's fee estimations. Eg. if Core estimates 6 hours at 10 sat/b, based on the past, but the mempool is already filled with 12 hours of 11 sat/b estimations, then what we do (currently with Knots) is that based on the mempool histogram we upgrade Core's estimations accordingly. This'd mean we're finally considering the PRESENT
• Finally, for future work we can also aim somehow at predicting the FUTURE (neuro net, AI stuff? Update: I think this repo by @djkazic wanna do something like this: https://github.com/djkazic/matrix)

@nopara73 I'm aware of estimatesmartfee's limitations, and understand the desire for something based on current mempool composition that can react faster to current conditions. The problem is that looking your own mempool only gives a reliable result in case you're running with policies that more or less match what miners are doing, and if not, becomes easily gameable. That is the reason why estimatesmartfee intentionally does not use mempool composition.

As I said, I think an RPC like this is valuable for people who want additional information for fee estimation, but using it automatically without at least someone looking at the result seems dangerous to me. E.g. future softforks your node doesn't know about could permit an attacker to stuff your mempool with high-fee unminable transactions.

@sipa By not providing sort-of accurate estimations from the current mempool, we force people to rely on third-party estimators, which is arguably much worse than having an independent estimation which works most of the time, although it may be games under certain specific conditions. I think that "perfect is enemy of good" rule applies here perfectly.

@sipa sorry for the late reply, at first I was like "yes, that makes perfect sense," however an argument came to my mind - this morning during shower :) - in favor of it, so I'd like to share that.

Can't we assume miners are seeking the highest fee transactions at all times? In other words: maximize their income from fees. If we could, then adjusting estimatesmartfee results based on the possibility of block acceptance of the current mempool state of the node does not seem gameable to me, because by extension we could assume that the highest fee transactions we have are precisely the ones that the miners seek.

Now I may argue against this with "it is not guaranteed that the miners have the highest fee txs that my node does as well."

To this, first I'd say: we can guarantee they want to have them and isn't that enough?
Then I'd turn this around and look at it from an attacker's point of view: the way to game this would be to create transactions those have high fees, get to the targeted node, but don't get to the miners. And now I'd point out the attacker cannot guarantee his high fee transactions don't get to the miners, especially with fullrbf around. This makes the attack super expensive, since in order to make the node think the fees are very high, the attacker has to create many blocks of transactions and guarantee that only the targeted node will get to know of these transactions.

In summary, unless I am missing the point and you're referring to another kind of game/attack here that does not involve the attacker creating blocks of high fee transactions, we can be reassured the attack is too expensive to be worth trying.

Using getrawmempool true is unfortunately not a solution that can be applied generally to solve this use case. On systems with low memory (4Gb or less, for example a Raspberry Pi or the Futurebit Apollo) this RPC call causes the kernel to terminate the Bitcoin Core process (OOM) when the mempool is reasonably full. This occurs consistently on all released versions AFAIK.

One workaround is to use getrawmempool false followed by repeated calls to getmempoolentry. This of course is even more inefficient, leading any reasonable client implementation to maintain a replica of mempool entries to avoid having to load them all, one by one, every time a fee rate histogram is required. This trades better performance for significantly increased memory utilisation.

It's a pity the PR has been closed since the complexity is low, there is clearly wide interest for it across many wallets, and a fee rate histogram is a useful tool in the toolbox when estimating fees. In addition, any middleware implementing the Electrum protocol needs to use one of the inefficient methods described to implement the mempool.get_fee_histogram call.

Request to all developers including maintainers to re consider their opinion.

+1.

I think the discussion here is conflating two things:

• Problems with Bitcoin Core's fee estimation algorithm.
• The lack of a fee histogram RPC.

It's clear that the existing fee estimation algorithms make it inaccurate for many real use cases. There are good reasons for why it works the way it does, but that doesn't change the fact that the outcome is bad. We can certainly discuss what can be done about that. I've posted about one possible approach in #27995.

However, this PR is about something else; adding a feerate histogram. That seems to me like a vaguely reasonable, independent feature, but it has nothing to do with our fee estimation algorithm. And wanting a feerate histogram so you can implement fee estimation yourself just because the fee estimation feature itself does a bad job seems to be the wrong way around. If that's the goal we should just think about how to improve the algorithm.

To be clear, my desire for an efficient way to retrieve fee rate histogram data has nothing to do with Bitcoin Core's fee estimation algorithm. Because fee estimation is inherently difficult and no algorithm can know the user's time preference for every transaction, Sparrow Wallet displays the fee rate histogram chart in the UI as another "tool in the toolbox". This allows a user to observe trends and adjust the fee rate to place a transaction in a particular fee rate band to suit their time preference. This is not to suggest such an approach is always superior to algorithmic fee rate estimation, merely that more data is generally useful when trying to predict the future.

One workaround is to use getrawmempool false followed by repeated calls to getmempoolentry. This of course is even more inefficient, leading any reasonable client implementation to maintain a replica of mempool entries to avoid having to load them all, one by one, every time a fee rate histogram is required. This trades better performance for significantly increased memory utilisation.

I think the getmempoolentry can be batched for a slight improvement. Also, you don't have to store the full entry in the application, just the txid and fee info would be enough? Finally, you'd only have to call getmempoolentry once per transaction.

Even if this pull was merged, it will just give one answer and I don't think it is clear whether this is the answer that users actually want? See #21422 (comment) and the previous discussion threads.
So anyone who wanted to give a better answer would have to implement some smarter algorithm in the application on a full copy of the mempool anyway, and this RPC wouldn't help with that.

Sparrow Wallet displays the fee rate histogram chart in the UI as another "tool in the toolbox". This allows a user to observe trends and adjust the fee rate to place a transaction in a particular fee rate band to suit their time preference.

a fee rate histogram is a useful tool in the toolbox when estimating fees

I think it's worth noting that a feerate histogram built from individual transaction feerates can be very misleading for fee estimation. A CPFP could look like n low feerate data points + 1 extremely high feerate data point. A few dozen 100kvB high-feerate transactions may dominate the block you are trying to get into, but they look identical to 100vB transactions as data points. As a user, I have no idea how I would look at a feerate histogram and get an idea of what feerate to put on my transaction.

I agree that fee estimation using the current contents of the mempool can be very useful. However, the process of taking current mempool contents and meaningfully translating them into high/med/low priority bands is quite involved. Linking one implementation which is a lot more accurate than using individual feerates, but quite computationally heavy and not necessarily appropriate for the oom'd raspis either.

Perhaps with cluster mempool we can expose a mempool entry's mining score, but that score still doesn't tell you much about how much block space is taken up by each data point.

leading any reasonable client implementation to maintain a replica of mempool entries

This is the most appropriate solution 👍. A simple map from txid to feerate would do if you're trying to keep a feerate histogram.

Also, you don't have to store the full entry in the application, just the txid and fee info would be enough? Finally, you'd only have to call getmempoolentry once per transaction.

Yeah, the nice thing about only needing fee and vsize is these will never change for a given txid in your mempool. You only need to query the entries you don't already have in your external data structure.

I think the getmempoolentry can be batched for a slight improvement.

On Core's side this idea seems the most worth pursuing, if somebody wants to open a pull.

Hey, developer of matrix here.

I think a neural net fee estimator is going to be pretty performant compared to mempool space. My logger already takes into account things like CPFP and calculates the total package fee rate, and with approx 50MB of training data it's capable of estimating fees such that if the necessary rate goes down, it reacts more quickly that estimatesmartfee.

Like the built-in fee estimator, I do not use the current contents of the mempool. Rather, the training data packages up the time of tx seen, block count from seen to confirmed, and total package feerate. When it comes to inference-time, lighter nodes like RPis can download and validate a pretrained model. Hence, no fear of OOMing.

The points regarding CPFP etc are well noted, although it's possible these are in practice more applicable to higher time preference fee estimations? Estimation aside, there are not insignificant client costs to maintaining a "txid to vsize and fee rate" mempool replica. By my measurement, this data structure with ~125,000 transactions requires ~14Mb of RAM.

Further, the initial load using getmempoolentry takes around 20 seconds on a locally running node, and would certainly be slower if the node was remote or running on an RPi. Because of this, I find it better to measure the time required for getrawmempool false and make a decision on whether to risk calling getrawmempool true instead of the getmempoolentry workaround. If a fee rate histogram RPC is not likely, I'd certainly be interested in a better solution to this.

By my measurement, this data structure with ~125,000 transactions requires ~14Mb of RAM.

If memory usage is the only concern, I am sure you could toss the transactions into (lossy) buckets and then subscribe via zmq to avoid duplicates, without having to remember txids, or individual size/fee details.

... applicable to higher time preference fee estimations?

If you only care about the top N blocks worth of transactions, the memory usage likely could also be reduced.

I don't see the use of the histogram for fee estimation. You can have a lot of transactions with minimum fee that generate a large bar dominating the histogram. I see more useful a representation like mempool.space showing the list of transactions ordered by decreasing fee and grouped by blocks. That is a list of pending blocks with associated feerate ranges.

@santochibtc "Feerate histogram" here is a bytes vs fee diagram, not a #tx vs fee diagram.

Thanks @sipa, yes, it is "a histogram of the fee rates paid by transactions in the memory pool, weighted by transaction size", but my point is still valid. If you want to include your tx in the next block you must estimate your fee based on the pending txs that are paying more per byte. I think that the histogram is not always a good picture of the mempool to estimate fees.

It's good enough.